Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add specs for MachData#deconstruct_keys and #deconstruct #1030

Merged

Conversation

AI-Mozi
Copy link
Contributor

@AI-Mozi AI-Mozi commented May 16, 2023

#1016
[Feature #18821]

MatchData#deconstruct has been added.
MatchData#deconstruct_keys has been added.

@AI-Mozi AI-Mozi force-pushed the add_specs_for_matchdata_deconstruct branch from 0f988fa to 276fd74 Compare May 16, 2023 06:37
@AI-Mozi
Copy link
Contributor Author

AI-Mozi commented May 16, 2023

I'm not sure why for example :x as an argument to deconstruct_keys raises
wrong argument type Integer (expected Array) not
wrong argument type Integer (expected Array or nil)

Also wondering how to add alias for captures. Should I create file captures.rb in matchdata/shared, move there implementation of captures_spec.rb and use it with it_behaves_like?
But this alias in only for 3.2 version so not sure if its right.

@AI-Mozi AI-Mozi marked this pull request as ready for review May 16, 2023 08:43
@andrykonchin
Copy link
Member

Should I create file captures.rb in matchdata/shared, move there implementation of captures_spec.rb and use it with it_behaves_like?

Agree. It makes sense.

@andrykonchin
Copy link
Member

andrykonchin commented May 16, 2023

'm not sure why for example :x as an argument to deconstruct_keys raises
wrong argument type Integer (expected Array) not
wrong argument type Integer (expected Array or nil)

Looks like keys validation was changed and they check that keys is Array and elements are Symbols with Check_Type:

Check_Type(keys, T_ARRAY);
Check_Type(key, T_SYMBOL);

https://github.com/ruby/ruby/blob/master/re.c#L2425

@andrykonchin
Copy link
Member

Also I would add the following test cases:

  • it returns {} when there are no named captured groups at all
  • it returns {} when passed more keys than named captured groups

@AI-Mozi AI-Mozi force-pushed the add_specs_for_matchdata_deconstruct branch from 276fd74 to aafefb4 Compare May 16, 2023 14:04
core/matchdata/shared/captures.rb Outdated Show resolved Hide resolved
core/matchdata/deconstruct_keys_spec.rb Outdated Show resolved Hide resolved
core/matchdata/deconstruct_keys_spec.rb Outdated Show resolved Hide resolved
core/matchdata/deconstruct_keys_spec.rb Outdated Show resolved Hide resolved
@AI-Mozi AI-Mozi force-pushed the add_specs_for_matchdata_deconstruct branch from aafefb4 to 8ce0022 Compare May 17, 2023 08:35
@andrykonchin
Copy link
Member

Thank you for the specs!

@andrykonchin andrykonchin merged commit a4e51bc into ruby:master May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants